Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

only allocate memory if needed #50

Conversation

psychocoderHPC
Copy link
Member

On the first allocation in chunk_list the first chunk will be added only if needed.
This prevents memory is allocated if no allocation is required.

This PR is addressing the comment: michaelsippel#1 (comment)

Copy link
Member

@michaelsippel michaelsippel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this will not work as intended.
please let me think about this for a second I believe there is a simpler solution

@@ -297,7 +319,16 @@ struct AtomicList
return MutBackwardIterator{ old_head };
}

// append the first head item if not already exists
bool try_append_first_item( std::shared_ptr< ItemControlBlock > new_head )
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is still possible to append two items with try_append_first_item() , because it is still an ordinary append function just that it fails if two calls happen concurrently in such a way that one thread messes with the already read-out value of head.

If two threads call try_append_first_item() with such scheduling that the second thread initialized old_head after the first thread already wrote to head, the second thread will simply append another item after head.

it should be like

std::shared_ptr< ItemControlBlock > expected( nullptr );
std::shared_ptr< ItemControlBlock > const & desired = new_head;

std::atomic_compare_exchange_strong<ItemControlBlock>( &head, &expected, desired );

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes this was my intention but wrote it during a meeting and than I forgot to add the empty element as old. I will update the branch with your suggestion.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

added your suggestion and forced push the change

On the first allocation in `chunk_list` the first chunk will be added
only if needed.
This avoids that memory is allocated if no allocation is required.
@psychocoderHPC psychocoderHPC force-pushed the topic-atomicListAllocFirtItem branch from 6ce9f56 to d2a0d85 Compare December 13, 2023 14:29
@michaelsippel michaelsippel merged commit 1287c5b into ComputationalRadiationPhysics:dev Dec 13, 2023
1 check passed
@psychocoderHPC psychocoderHPC deleted the topic-atomicListAllocFirtItem branch December 14, 2023 07:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants